Skip to content

fix(dav): ignore calendars in the trash bin for free-busy and user status - #62667

Draft
ndo84bw wants to merge 2 commits into
nextcloud:masterfrom
ndo84bw:fix/32989-no-freebusy-from-deleted-calendars
Draft

fix(dav): ignore calendars in the trash bin for free-busy and user status#62667
ndo84bw wants to merge 2 commits into
nextcloud:masterfrom
ndo84bw:fix/32989-no-freebusy-from-deleted-calendars

Conversation

@ndo84bw

@ndo84bw ndo84bw commented Jul 29, 2026

Copy link
Copy Markdown

Summary

If a calendar holds events that count as busy and you delete that calendar, the events in it are still evaluated for free/busy lookups and for the automatic user status.

The reason is that deleting a calendar only marks the calendar as deleted, not the events inside it. Trashed calendars are hidden from clients purely through their resourcetype, but Sabre's free/busy code picks the calendars to query by PHP class, so that masking never applies there.

This patch filters out calendars in the trash bin in both cases.

What the fix does

  • Calendar::calendarQuery() returns [] when the calendar is in the trash bin. This covers the scheduling outbox free/busy request (Sabre's Schedule\Plugin::getFreeBusyForEmail), OCA\DAV\CalDAV\Schedule\Plugin::isAvailableAtTime (room and resource auto-reply), OC\Calendar\Manager::checkAvailability, and the calendar-query and free-busy-query REPORTs.
  • CalendarImpl::search() returns [] when the calendar is in the trash bin. This covers Manager::searchForPrincipal() and Manager::search(), and with them StatusService (the "In a meeting" user status) and UpcomingEventsService (dashboard and upcoming events).

What the fix does not touch

  1. PROPFIND Depth:1 on a trashed calendar still lists its objects, and GET still serves them with 200.
  2. Public links: after trashing, PROPFIND on /public-calendars/<token>/ still answers 207 and lists the object, while a calendar-query REPORT on the same link now comes back empty.
  3. CalDavBackend::getCalendarObjectByUID() filters co.deleted_at but not c.deleted_at, so an incoming iTIP reply can still be applied to an event inside a trashed calendar. Skipping it would duplicate the event instead: Sabre would treat it as unknown and create a new copy in the default calendar, next to the old one in the trashed calendar.

One consequence I could not verify: scheduleLocalDelivery auto-replies for rooms and resources based on the same availability query, so a room whose own calendar sits in the trash bin should now reply ACCEPTED instead of DECLINED, because the bookings in that calendar no longer count as busy. Reproducing this needs a room or resource principal, which only exists if an app provides one.

Tests

Manual, live (docker dev container, NC 35.0.0 dev, PHP 8.5):

  • Free/busy over curl against the outbox: a calendar with an event on 2026-07-29 10:00-11:00Z, queried by a second user. Before the fix, DELETE of the calendar still returned FREEBUSY:20260729T100000Z/20260729T110000Z; with the fix the VFREEBUSY comes back empty. Restoring from the trash bin (MOVE to trashbin/restore/file) still works, and the slot is busy again afterwards.
  • calendar-query REPORT on the trashed calendar: empty multistatus.
  • Free/busy in three places, tested A/B by me (patch off, patch on): "Find time" in the event dialog of the Calendar app, the meeting proposal view, and Thunderbird's attendee availability. Without the patch the busy time of the trashed calendar shows up everywhere, with the patch nowhere. That also shows that Thunderbird really does query the scheduling outbox.
  • Upcoming events API (GET /ocs/v2.php/apps/dav/api/v1/events/upcoming), A/B with and without the CalendarImpl guard: without it the event from the trashed calendar appears, with it the list is empty.
  • User status, tested A/B by me: a running event in a trashed calendar. Without the patch the status switches to busy with message_id = meeting, so the calendar icon shows on the avatar; with the patch revertUserStatus() restores the previous online status and the icon disappears. Two things to keep in mind when reproducing: the calendar status result is cached per user for 5 minutes (StatusService, where the cache lives depends on memcache.local), and a manually set status suppresses events that started before it, otherwise the test comes out falsely negative.

Unit tests:

  • CalendarTest covers calendarQuery() for a live, a trashed and a shared calendar (PublicCalendarTest inherits them)
  • CalendarImplTest covers search() for a trashed calendar
  • phpunit --filter "CalendarTest|CalendarImplTest" - 143 tests green

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI
    • reproducing the bug described in the issue
    • identifying the root cause
    • brainstorming the right place for a patch
    • writing the unit tests
    • manual tests on the CLI

ndo84bw added 2 commits July 29, 2026 12:03
@ndo84bw
fix(caldav): stop querying objects of calendars in the trash bin
f6f14f3 
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Nico Donath <ndo84bw@gmx.de>
@ndo84bw
fix(caldav): stop searching objects of calendars in the trash bin
d47d19a 
Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Nico Donath <ndo84bw@gmx.de>
@ndo84bw ndo84bw changed the title Fix/32989 no freebusy from deleted calendars fix(dav): ignore calendars in the trash bin for free-busy and user status Jul 29, 2026
SebastianKrupinski
SebastianKrupinski requested changes Jul 29, 2026
View reviewed changes

@SebastianKrupinski SebastianKrupinski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I am going to block this PR.

calendarQuery() and search() are used by other parts of our code to find objects both in and outside of the trash bin.

Not sure doing this here is the best idea

@ndo84bw

ndo84bw commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Hi @SebastianKrupinski ,

The Trashbin listing runs via a different path with a method of the same name. (Trashbin\DeletedCalendarObjectsCollection::calendarQuery())
And then there's also collectDeletedCalendarObjectsForPrincipal for finding events in a deleted calendar.

I’ve seen this in the following consumers:

Note RFC-6638 9.1: schedule-calendar-transp (opaque | transparent)

When deleting the calendar, we could set this and would have already handled the two prominent queries. However, then I wouldn’t know the previous state during restoration.

In addition to these two consumers, I’ve identified the following (not yet tested; there may be a few too many):

  • Upcoming events in the dashboard: UpcomingEventsService.php
  • Unified Search: CalDavBackend::searchPrincipalUri()
  • Public calendar links: getPublicCalendar
  • CalDAV sync
  • iTIP delivery getCalendarObjectByUID
  • Room and resource availability isAvailableAtTime
  • Event booking slots in the Calendar app (EventConflictFilter, DailyLimitFilter)

I mean, I read in the Contributor Guidelines that a review shouldn't take any more time than the value the patch adds.

This would be one of those cases for me. Issue #32989 describes a problem that I would call an edge case. And on top of that, it resolves itself automatically after a certain amount of time.

My suggestion:
When deleting the calendar, I’ll set schedule-calendar-transp=transparent, and when restoring it, I’ll set the system default schedule-calendar-transp=opaque, thereby handling the prominent consumers “Sabre Free-Busy” and “Nextcloud User Status.”
Writing an overwrite in “Sabre Free-Busy” for “deleted” would then require maintaining the function for the lifetime of Nextcloud - plus edit all other consumers, some of which are in different repositories.

Do you agree with my suggestion, or do you have another solution that would take less time? If not, I’ll close my PR. I don’t think it’s reasonable.

@ndo84bw
ndo84bw marked this pull request as draft July 31, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SebastianKrupinski SebastianKrupinski SebastianKrupinski requested changes
@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst ChristophWurst is a code owner
@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

FreeBusy shows appointments of deleted calendars

2 participants

@ndo84bw @SebastianKrupinski